Skip to content

Line graph on press fix#129

Merged
Hermanya merged 2 commits into
chart-kit:masterfrom
miwialex:line-graph-on-press-fix
Jul 6, 2019
Merged

Line graph on press fix#129
Hermanya merged 2 commits into
chart-kit:masterfrom
miwialex:line-graph-on-press-fix

Conversation

@miwialex

Copy link
Copy Markdown
Contributor

The current logic for the LineGraph component relies on the passed in dataSet color to use the opacity param in the function. If you want a solid color, and don't use the opacity param, this creates a huge dot on the screen. So I updated the larger click area dot, to not rely on the opacity of the passed in color, but set it on the circle itself.

I also noticed that pulling down the code and running it, the dots were not showing up on the line graph in iOS. I noticed this when I upgraded to the latest version in my project as well. iOS seems to not like the dots being inside a View, and removing them from that, seemed to work.

Last thing I updated was updating the app.json for development. If you don't have sdkVersion set, it uses the latest version installed (currently sdk 33 for me) instead of the one from package.json. This was throwing errors for me.

Ruben Manrique added 2 commits June 30, 2019 12:00
if sdkVersion is not present, expo will use the latest version installed on the computer vs the one from package.json
* change larger click circle to not rely on the fill to have opacity
* remove Circles from View that were making them not show up in iOS
@Hermanya Hermanya merged commit 5daa854 into chart-kit:master Jul 6, 2019
@Hermanya

Hermanya commented Jul 6, 2019

Copy link
Copy Markdown
Contributor

Thank you for your contribution 👍
It's now available in v3.0.1

@miwialex

miwialex commented Jul 8, 2019

Copy link
Copy Markdown
Contributor Author

awesome, glad I could help!

@miwialex miwialex deleted the line-graph-on-press-fix branch July 8, 2019 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants